Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: MesonToolchain: improve cross-compilation workflow on Apple systems #9711

Closed
wants to merge 1 commit into from

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Oct 2, 2021

closes: #9710

Changelog: Feature: MesonToolchain: improve cross-compilation workflow on Apple systems
Docs: omit

  • adds a mixin MesonAppleMixin to add some important flags specific to the Apple systems

  • adds missing obj[cpp]_[link]_args to the cross-file (compiler/linker flags for Objective-C and Objective-C++ languages)

  • adds missing root to the cross-file (sysroot)

  • new test for M1 cross-building

  • Refer to the issue that supports this Pull Request.

  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.

  • I've read the Contributing guide.

  • I've followed the PEP8 style guides for Python code.

  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@SSE4 SSE4 force-pushed the meson_cross_apple branch 2 times, most recently from c257a2f to 354d88d Compare October 2, 2021 12:35
self.objcpp_link_args.extend(args)


class MesonToolchain(MesonBaseToolchain, MesonAppleMixin):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, no multiple inheritance. And avoid inheritance as much as possible. Go with composition unless there is an extremely compelling case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a well-known mixin pattern?
/cc @jgsogo
suggestion on how to to it better are welcomed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@memsharded do you want me to implement AppleSystemBlock like it was done in CMakeToolchain?
https://github.com/conan-io/conan/blob/develop/conan/tools/cmake/toolchain.py#L280

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please, avoid inheritance and mixins unless it is strictly necessary. Composition scales better and is more flexible for user customization. Going with a "blocks" approach like the CMakeToolchain will be better, please go that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am looking at how to implement blocks similarly to how it was done in CMake toolchain.
there are several ideas, and I think I'll need some advice.
first off, there are two main things Apple block should define:

  • compiler executables, c (known as CC in autotools world) and cpp (known as CXX)
  • flags for compilers and linkers, like c_args (CFLAGS), cpp_args (CXXFLAGS), link_args (LDFLAGS), etc.

for executables, it should take the very least priority (only use when not explicitly defined), for variables, it should effectively append them to any existing.

then, unlike CMake, meson machine files are simple .ini files with very limited syntax. they are not turing-complete, and cannot contain conditions, loop, and other flow control instructions. there are very few operators supported, like + and / (https://mesonbuild.com/Machine-files.html#constants), but that's it.

on the other hand, there is an interesting feature that meson supports multiple cross-files overriding each other: https://mesonbuild.com/Machine-files.html#loading-multiple-machine-files

so, we may use something like --cross-file conan_meson_cross_apple.ini --cross-file conan_meson_cross_main.ini.

therefore, there are likely 3 major approaches on how to implement that:

  1. use multiple cross-files overriding each other
  2. use a single cross-file with several blocks
  3. use a single cross-file with one block, but compose several contexts within the toolchain
  1. is easy to implement, but it might be kinda misleading: too many files to deal with for the consumer, e.g. too long command lines, and easy to mess up with an order
  2. might be tricky with a limited .ini syntax. e.g. I can't define the same value twice within the same machine file, I will get an error:
configparser.DuplicateOptionError
option 'buildtype' in section 'built-in options' already exists

and I can't check if it's already defined above, without using an external check in the code.

  1. is also easy to implement, it will be just a matter of creating several context dictionaries and merging them according to our specific rules (executables override, flags append). but it's not really a blocks approach.

looking for the advice, thanks!

if not is_apple_os(conanfile.settings.get_safe("os")):
return

xcrun = XCRun(conanfile.settings)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better move XCRun first to modern approach. It should be less detection based and more configuration based.

Copy link
Contributor Author

@SSE4 SSE4 Oct 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the XCRun is already imported from a new namespace conan.tools.apple or what exactly do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyway, I think improvements of XCRun, if any, could be done in another, their own PR. this is outside of the scope of this PR IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that generators like this one do not auto-detect anything. XCRun is deducing things by executing some commands right? That should be moved to input configuration, like the new [conf].

This is a general effort we are doing in 2.0, we are removing all the platform, architecture, compiler, env-vars detection in generators and helpers, and that should happen in the input configuration. Please try that approach better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know right now how it should look like and how it should be implemented.
if you have some example, I can take a look, but my point it belongs to XCRun, not to the meson toolchain.
in other words, as I understand, XCRun will have some code to read conf file instead of executing commands.
I don't see any reason why it should be done inside this particular PR - it is just using XCRun, and for meson toolchain it's an implementation detail how exactly it is implemented under the hood.
I also don't understand how is it going to work in CI, e.g. in case of ConanCenter, e.g. who, when and how exactly should populate conf with the input configutation?
really, I am just relying on the existing implementations of conan.tools namespace here, if they are somehow bbad/buggy or don't meet the conan 2.0 requirements, let's create an another issue and improve in another PR.

@SSE4 SSE4 changed the title - MesonToolchain: improve cross-compilation workflow on Apple systems Feature: MesonToolchain: improve cross-compilation workflow on Apple systems Oct 8, 2021
@SSE4 SSE4 marked this pull request as ready for review October 26, 2021 15:30
@SSE4
Copy link
Contributor Author

SSE4 commented Oct 26, 2021

I would love to see this moving forward
the bad things are:

  • old meson build helper doesn't support cross-building at all
  • new meson build helper doesn't work for M1 out of the box

we have a plenty of foundational recipes using meson as a build system, e.g.:

  • GLib
  • Cairo
  • Pango
  • GStreamer and its plug-ins

@wbehrens-on-gh
Copy link

@SSE4 Does the new meson build helper only fail to cross compile to M1 or does it fail to cross build in all cases?

@SSE4
Copy link
Contributor Author

SSE4 commented Nov 11, 2021

@SSE4 Does the new meson build helper only fail to cross compile to M1 or does it fail to cross build in all cases?

no, not just Apple. see issue #9922 for the similar incident with Visual Studio cross-compilation (x86_64 -> x86).
in general, Meson, unlike CMake, cannot deduce a compiler executable in case of cross-compiling.

@SSE4 SSE4 requested a review from memsharded November 12, 2021 08:35
@franramirez688
Copy link
Contributor

In the end, it was superseded by #10174

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] MesonToolchain: improve cross-compilation workflow on Apple systems
4 participants